Skip to content

refactor(aarch64,riscv64): unify kernel image detection#607

Merged
mkroening merged 2 commits into
hermit-os:mainfrom
Gelbpunkt:initrd-start-aarch64
Jun 11, 2026
Merged

refactor(aarch64,riscv64): unify kernel image detection#607
mkroening merged 2 commits into
hermit-os:mainfrom
Gelbpunkt:initrd-start-aarch64

Conversation

@Gelbpunkt

@Gelbpunkt Gelbpunkt commented May 15, 2026

Copy link
Copy Markdown
Member

This unifies the kernel image detection for both aarch64 and riscv64. riscv64 was incorrectly using the length of the module reg as the size of the kernel image, which was always 0, which is fixed by using the approach taken by the aarch64 code to parse the ELF header. On the other hand, aarch64 now falls back to linux,initrd-start and linux,initrd-end for detecting the kernel image thanks to these changes, which is necessary for cloud-hypervisor at the moment.

@mkroening mkroening left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have similar code for RISC-V: https://github.com/hermit-os/loader/blob/v0.5.6/src/arch/riscv64/mod.rs#L21-L51

It might make sense to unify them as methods on an trait FdtExt, but that might not be worth the effort. You can decide. :)

@Gelbpunkt

Copy link
Copy Markdown
Member Author

Ah, I didn't even bother checking the RISC-V code. That one is also slightly prettier than this approach. I'll try the FdtExt approach, that seems sensible.

@Gelbpunkt Gelbpunkt force-pushed the initrd-start-aarch64 branch from 80527af to baa7cc9 Compare May 21, 2026 21:20
@Gelbpunkt Gelbpunkt changed the title feat(aarch64): Fall back to detecting kernel at linux,initrd-start refactor(aarch64,riscv64): unify kernel image detection May 21, 2026
@Gelbpunkt Gelbpunkt force-pushed the initrd-start-aarch64 branch from baa7cc9 to 3725077 Compare May 21, 2026 21:21
@Gelbpunkt Gelbpunkt requested a review from mkroening May 21, 2026 21:22
@Gelbpunkt

Gelbpunkt commented May 21, 2026

Copy link
Copy Markdown
Member Author

CI on aarch64 fails because the fdt is... invalid as far as I can tell. size-cells is set to 2 and address-cells set to 2, but the inserted module only has 32-bit components, i.e. is for size-cells 1 and address-cells 1. Not sure yet what's responsible for this, but it was already an issue before and the code just didn't properly parse/validate it

@Gelbpunkt Gelbpunkt force-pushed the initrd-start-aarch64 branch from 3725077 to 5ce1b37 Compare May 24, 2026 00:23
@Gelbpunkt

Copy link
Copy Markdown
Member Author

That's fixed now, we were the ones committing crimes with the devicetree in xtask :)

@mkroening mkroening self-assigned this May 24, 2026
This unifies the kernel image detection for both aarch64 and riscv64.
riscv64 was incorrectly using the length of the module reg as the size
of the kernel image, which was always 0, which is fixed by using the
approach taken by the aarch64 code to parse the ELF header. On the other
hand, aarch64 now falls back to linux,initrd-start and linux,initrd-end
for detecting the kernel image thanks to these changes, which is
necessary for cloud-hypervisor at the moment.
The generated devicetree contains #address-cells = <2> and #size-cells =
<2>, which means that address and size are expected to be 64-bit values,
but we were currently only constructing two 32-bit values, which meant
that effectively, when parsing it, the size was missing. Fix it by
making the values 64-bit wide.
@Gelbpunkt Gelbpunkt force-pushed the initrd-start-aarch64 branch from 5ce1b37 to 938ff80 Compare June 11, 2026 14:09

@mkroening mkroening left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :)

@mkroening mkroening added this pull request to the merge queue Jun 11, 2026
Merged via the queue into hermit-os:main with commit c773c39 Jun 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants